-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next 정진호 sprint8 #57
base: next-정진호
Are you sure you want to change the base?
The head ref may contain hidden characters: "next-\uC815\uC9C4\uD638-sprint8"
Next 정진호 sprint8 #57
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
진호님 고생많으셨습니다!
몇가지 코멘트 남겨놨는데, 확인부탁드려요~
이해 안가시는 부분이 있으시면 채널 혹은 멘토링때 물어보셔도 됩니다!
수고하셨습니다~
} = {}) => { | ||
const url = `/products?page=${page}&pageSize=${pageSize}&orderBy=${orderBy}&keyword=${keyword}`; | ||
const response = await client.get(url); | ||
const data = response.data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런건 그냥 간결하게 return response.data 로 빼도 될것 같아요~
async function ArticlePage(props) { | ||
const params = await props.params; | ||
const articleId = params.articleId; | ||
const article = await api.getArticle(articleId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 api를 호출하게되면 해당 api를 받아올떄까지 UI를 그리지 않아 사용자 경험이 안좋을 수 있습니다. useEffect 로 빼서, 로딩이나 초기값을 넣어놓고 렌더후 그다음 업데이트를 하는게 더 나을것 같아요~
import Link from "next/link"; | ||
import ArticleComments from "./_components/ArticleComments"; | ||
import IconHeart from "@/components/IconHeart"; | ||
import DeleteArticleButton from "./_components/DeleteArticleButton"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_components 와 @/components 의 차이가 무엇인가요?? @/components 가 단순히 common 역할을 하는것보다
@/components/common
@/components/articles/DeleteArticleButotn 이렇게 하는게 더 통일성 있을것 같아요
const article = await api.getArticle(articleId); | ||
|
||
const isoDate = article.createdAt; | ||
const formattedDate = dayjs(isoDate).format("YYYY.MM.DD"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isoDate로 따로 변수를 빼야하는지는 모르겠습니다. article.createdAt은 한번만 쓰여서요! 차라리 포맷팅을 동일하게 하는 지점들을 찾아 format util 로 빼어 간결하게 하는게 더 좋을것 같습니다!
import React from "react"; | ||
|
||
function ArticleCard({ item }) { | ||
const isoDate = item.createdAt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지입니다!
|
||
const handleSubmit = async (e) => { | ||
e.preventDefault(); | ||
if (!title && !content) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조건이 둘다 없으면이 아니라 둘중 하나라도 없으면 이 맞을것 같아요~
</div> | ||
<div className="flex flex-col gap-4"> | ||
{filteredArticles.map((item) => { | ||
return <ArticleCard key={item.id} item={item} />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
item.id 가 숫자라면 더더욱 unique 키에 걸릴수 있으니 article-${item.id}
같이 사용하는게 더 좋습니다! 같은 페이지에 숫자 키를 쓰는 컴포넌트가 있으면 문제가 되니까요~
요구사항
자유 게시판 페이지
게시글 등록 & 수정 페이지
게시글 상세 페이지
심화 요구사항
공통
스크린샷
-/articles/[articleId] 게시글 상페 화면 (댓글 기능 구현 x)

멘토에게
따라서 '홈 화면'과, '상품 화면'은 구현되어 있지 않습니다.